-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(toolkit): add 'cdk context' command #1169
Conversation
Add a command to view and manage cached context values. Fixes #311.
docs/src/context.rst
Outdated
Viewing and managing context | ||
########################### | ||
|
||
Context is used to retrieve things like Availability Zones available to you, or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Availability zone available" sounds weird
docs/src/context.rst
Outdated
│ │ 64-gp2:region=us-east-1 │ │ | ||
└───┴────────────────────────────────────────────────────┴────────────────────────────────────────────────────┘ | ||
|
||
Run cdk context --invalidate KEY_OR_NUMBER to invalidate a context key. It will be refreshed on the next CDK synthesis run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I wonder if
--clear
or--reset
are more intuitive in this context - Can you invalidate everything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--clear
actually clears everything. cdk context --help
will show it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will call it reset.
return 0; | ||
} | ||
|
||
function listContext(context: any) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a --json
flag that will output the context as JSON for machines?
} | ||
|
||
// Unset! | ||
if (!(key in context)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably be a no-op (think scripts that want to ensure that a context is invalidated)
return key; | ||
} | ||
} | ||
throw new Error(`No context key with number: ${n}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add --force and this will be no-op (similar to rm -f
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for a semi-interactive usage, where copying the key is a bother and you simply supply the number. Blindly deleting by key # is dangerous, given that the numbering is not unique.
But yeah, just deleting a nonexistant key will be a no-op.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right of course!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last question... can you write a small integration test for this and make sure you execute all previous tests as well?
Yas |
Add a command to view and manage cached context values.
Fixes #311.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.